Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve query capabilities #932

Merged
merged 5 commits into from
Sep 21, 2023
Merged

Improve query capabilities #932

merged 5 commits into from
Sep 21, 2023

Conversation

iand
Copy link

@iand iand commented Sep 19, 2023

Rework the Coordinator's Query method into two separate versions:

QueryClosest attempts to find the closest nodes to a key and returns the ones it finds
QueryMessage iterates closer nodes using a supplied message

Also allows the NumResults query config option to be specified when starting a query. This specifies the minimum number of nodes to successfully contact before considering iteration complete. A query is considered to be exhausted when it has received responses from at least this number of nodes and there are no closer nodes remaining to be contacted.

Also implements the basic version of DHT.GetValue and a simple test that fetches a value from another node in the network using a query.

@iand iand added the v2 All issues related to the v2 rewrite label Sep 20, 2023
v2/coord/coordinator.go Outdated Show resolved Hide resolved
ctx, cancel := context.WithCancel(ctx)
defer cancel()

seeds, err := c.GetClosestNodes(ctx, msg.Target(), 20)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should make 20 configurable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this number is like a global parameter. If we set this here to e.g. 30, every other peer would still respond with the 20 closest peers it knows. The remaining ten that we receive are just other random close peers. @guillaumemichel devised a great technique to query for a wider range of peers which is more elaborate: https://www.notion.so/pl-strflt/N-closest-peers-83038476d00f4af1bfb40aeeefec39f5?pvs=4

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should be configurable. Though here the call to GetClosestNodes is getting the 20 closest nodes from our own routing table to seed the query.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indirectly used the DHT's bucketsize config value

@iand iand self-assigned this Sep 20, 2023
* Move coord and kadt packages to internal

* go mod tidy

* go fmt

* Move kadt out of internal and add RoutingTable interface
@iand iand merged commit 2da54ab into v2-develop Sep 21, 2023
11 checks passed
@dennis-tra dennis-tra deleted the v2-routing-getvalue2 branch September 21, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 All issues related to the v2 rewrite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants